-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(createProject): add types and remove useless projectForm arg… #17
Conversation
@Daniel-Boll @juliano-soares please review the PR,: 1 - Test with npm pack and install it globally Waiting your return for approval and publishing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!!
src/new/form.ts
Outdated
packageManager: string; | ||
template: Template; | ||
confirm: boolean; | ||
}; | ||
const projName: string = projectName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not related to your changes, but thinking enough about it, this variable is useless this way, we can already use the projectName
it self, can you add this change as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
2f4f41b
to
37d04eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the argument order code will delete the option of the user silently install the application at any order. Your code change only allow users to create projects in this form:
expressots new app-demo -p npm -t opinionated
Our current solution allows the user to pass the -p (package manager) and -t (template) at any order
Well done, work completed! |
…s iteration
Pull Request Guidelines
Our guidelines for submitting a pull request.
Before submitting a Pull Request, please make sure you have verified the following:
fix typo in README.md
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Please describe the current behavior that you are modifying, or link to a relevant issue.
Issue Number: N/A
What is the new behavior?
Describe the new behavior or link to a relevant issue.
Does this PR introduce a breaking change?
If this PR contains a breaking change, please describe the impact and migration path for existing applications below.
Other information
Since the projectForm already sends in the order of the arguments correctly, to set the variables, it is not necessary to use an iteration, just use the deconstructure